feat(ink-compat): add getInnerHeight and Box scroll compatibility#31
Conversation
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: af9ecf36a9
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| mapped.overflowY === "scroll" | ||
| ? applyVerticalScroll(childVNodes, mapped.scrollTop) |
There was a problem hiding this comment.
The new overflowY="scroll" path always applies applyVerticalScroll before rendering children, regardless of stackKind. In a row container, that child list is horizontal columns, so any positive scrollTop removes leading columns instead of vertically clipping rows (for example, two 1-line row children with scrollTop={1} will render only the second child). This makes vertical scrolling corrupt row layouts and should be gated to column flow (or replaced with axis-correct clipping).
Useful? React with 👍 / 👎.
| const h = Math.max(1, Math.round(child.estimatedHeight)); | ||
| if (offset < h) break; | ||
| offset -= h; |
There was a problem hiding this comment.
applyVerticalScroll only skips a child when scrollTop is at least that child's full estimated height, and breaks otherwise. If the first child is multiline (height > 1), values like scrollTop={1} leave that child fully visible, so line-by-line scrolling inside a child never happens. This creates visibly incorrect viewport behavior for mixed-height content and needs partial-offset handling instead of whole-child dropping only.
Useful? React with 👍 / 👎.
2 participants
Summary
This PR closes the remaining Gemini-critical Ink compatibility gaps in
@rezi-ui/ink-compatby:getInnerHeightto the public API and measurement implementation.overflowY="scroll"(andoverflow/overflowXunions updated to includescroll)scrollTop/scrollLeftscrollbarThumbColoroverflowY="scroll"+scrollTop.scrollHeight,clientHeight) now account for scroll containers and borders.ink-spinnerink-gradientBefore / After
Before
getInnerHeightwas missing from exports.overflowY="scroll"was not supported in Box props/types, andscrollTop/scrollbarThumbColorwere not Ink-shaped in compat types.After
getInnerHeight(node)is exported and returns client/inner-height semantics (content area excluding border).scroll,scrollTop,scrollbarThumbColor).overflowY="scroll".ink-spinnerandink-gradient.Test Evidence
Validation run from a clean install in this branch:
npm ci-> PASSnpm run build-> PASSnpm test-> PASStests: 935pass: 935fail: 0Added/updated targeted tests in
packages/ink-compat/src/__tests__:measurement.test.tsxgetInnerHeightcorrectness across rerenders.overflowY="scroll"+scrollTopviewport behavior (Gemini-like case).scrollbarThumbColorexplicit no-op contract (accepted, non-breaking, scroll metrics unaffected).props.test.tsapi.surface.test.tsgetInnerHeight.compat.thirdparty.test.tsxink-spinnerruntime frame updates.ink-gradientTransform integration and render compatibility.Third-party packages tested
ink-spinner@5.0.0ink-gradient@3.0.0ink@4.4.1Known unverified 3rd-party packages
Not in scope for this PR (follow-up recommended):
ink-text-inputink-select-inputink-tableink-big-textRemaining Known Incompatibilities / Follow-up
scrollbarThumbColoris currently an accepted compatibility no-op in this renderer path (documented/tested contract), not a full visual scrollbar skin implementation.Text/Transform/simple Box scroll semantics.